Adding nginx_proxy access_log format ability #4102

tataruty · 2025-10-17T12:02:10Z

Proposed changes

Problem: As a user of NGF
I want to have the ability to configure the log format of NGINX's access and error logs
So that I can easily collect logs from NGINX in my logging platform.

Solution: Adding accessLog.Disabled and accessLog.Format fields to nginx-proxy CRD, also adding default values for them

Testing:
Tested with custom format:

kubectl patch nginxproxy nginx-gateway-proxy-config -n nginx-gateway --type merge -p '{"spec":{"logging":{"errorLevel":"debug","accessLog":{"format":"$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent"}}}}'

result:
config uses /dev/stdout and default format name:

work:

Tested with turning logs OFF:

kubectl patch nginxproxy nginx-gateway-proxy-config -n nginx-gateway --type merge -p '{"spec":{"logging":{"errorLevel":"debug","accessLog":{"disabled":true}}}}'

result:
config has only off without custom log_format:

Closes #1200

Checklist

Before creating a PR, run through this checklist and mark each as complete.

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have checked that all unit tests pass after adding my changes
I have updated necessary documentation
I have rebased my branch onto main
I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

Add ability to configure access log format or turn logging off

codecov · 2025-10-17T12:04:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.14%. Comparing base (96032ac) to head (7022c4f).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4102      +/-   ##
==========================================
+ Coverage   86.09%   86.14%   +0.04%     
==========================================
  Files         131      131              
  Lines       14171    14193      +22     
  Branches       35       35              
==========================================
+ Hits        12200    12226      +26     
+ Misses       1767     1764       -3     
+ Partials      204      203       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

apis/v1alpha2/nginxproxy_types.go

internal/controller/provisioner/objects.go

apis/v1alpha2/nginxproxy_types.go

internal/controller/state/dataplane/types.go

apis/v1alpha2/nginxproxy_types.go

internal/controller/nginx/config/base_http_config_template.go

internal/controller/state/dataplane/configuration.go

apis/v1alpha2/nginxproxy_types.go

internal/controller/nginx/config/base_http_config_template.go

apis/v1alpha2/nginxproxy_types.go

ciarams87

The API definition feels a bit confusing - as we are only supporting a single log format and a single path, we should simplify the API.

The two-level structure (NginxLogFormat + NginxAccessLog) creates indirection:

Users define a format with a name
Then reference that name in accessLog
This can lead to misconfiguration (mismatched names)
Both LogFormat AND AccessLog must be set for anything to work

I think we can remove the NginxLogFormat type completely and change the Format field in NginxAccessLog to specify the format string directly. We don't need to provide a mechanism for the user to specify the name of the log format, we can define that as an internal string.

Something like:

type NginxAccessLog struct {
    // Disabled turns off access logging when set to true.
    // +optional
    Disabled *bool `json:"disabled,omitempty"`
    
    // Format specifies the custom log format string.
    // If not specified, NGINX default 'combined' format is used.
    // See https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format
    // +optional
    Format *string `json:"format,omitempty"`
}

Later, if we decide to support different path types (e.g. syslog) we can extend this type to add the path field.

Then, if the access log format is set the template renders something like:

log_format ngf_default '<user-format>';
access_log /dev/stdout ngf_default;

And if it's disabled,

access_log off;

Otherwise we can omit the directive, and NGINX will use the default /dev/stdout combined (same behaviour as we have currently)

Does that make sense?

apis/v1alpha2/nginxproxy_types.go

internal/controller/nginx/config/base_http_config_test.go

internal/controller/nginx/config/base_http_config.go

internal/controller/nginx/config/base_http_config_template.go

internal/controller/nginx/config/base_http_config.go

internal/controller/state/dataplane/configuration.go

internal/controller/nginx/config/base_http_config.go

internal/controller/nginx/config/base_http_config_template.go

ciarams87 · 2025-11-07T16:19:11Z

Looks pretty good, good job!

We should also add the new fields to the Helm schema https://github.com/nginx/nginx-gateway-fabric/blob/main/charts/nginx-gateway-fabric/values.yaml#L451

github-project-automation bot added this to NGINX Gateway Fabric Oct 17, 2025

github-project-automation bot moved this to 🆕 New in NGINX Gateway Fabric Oct 17, 2025

github-actions bot added the enhancement New feature or request label Oct 17, 2025

tataruty commented Oct 17, 2025

View reviewed changes

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

apis/v1alpha2/nginxproxy_types.go Show resolved Hide resolved

internal/controller/provisioner/objects.go Outdated Show resolved Hide resolved

sjberman reviewed Oct 17, 2025

View reviewed changes

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

tataruty force-pushed the enhancement/configurabe_dataplane_log_format branch from 0923130 to 3d18022 Compare October 24, 2025 18:28

tataruty marked this pull request as ready for review October 24, 2025 18:30

tataruty requested a review from a team as a code owner October 24, 2025 18:30

tataruty commented Oct 24, 2025

View reviewed changes

internal/controller/state/dataplane/types.go Show resolved Hide resolved

sjberman reviewed Oct 24, 2025

View reviewed changes

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

internal/controller/nginx/config/base_http_config_template.go Outdated Show resolved Hide resolved

internal/controller/state/dataplane/configuration.go Show resolved Hide resolved

tataruty force-pushed the enhancement/configurabe_dataplane_log_format branch 3 times, most recently from 253e3be to b68f73f Compare November 3, 2025 11:17

tataruty requested a review from ciarams87 November 3, 2025 12:58

shaun-nx reviewed Nov 3, 2025

View reviewed changes

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

salonichf5 reviewed Nov 3, 2025

View reviewed changes

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

salonichf5 reviewed Nov 3, 2025

View reviewed changes

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

salonichf5 reviewed Nov 3, 2025

View reviewed changes

internal/controller/nginx/config/base_http_config_template.go Show resolved Hide resolved

salonichf5 reviewed Nov 3, 2025

View reviewed changes

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

tataruty force-pushed the enhancement/configurabe_dataplane_log_format branch from b68f73f to 74bb187 Compare November 4, 2025 11:16

ciarams87 requested changes Nov 5, 2025

View reviewed changes

github-project-automation bot moved this from 🆕 New to 🏗 In Progress in NGINX Gateway Fabric Nov 5, 2025

tataruty force-pushed the enhancement/configurabe_dataplane_log_format branch from 74bb187 to aadd4f6 Compare November 6, 2025 11:11

ciarams87 reviewed Nov 6, 2025

View reviewed changes

apis/v1alpha2/nginxproxy_types.go Outdated Show resolved Hide resolved

tataruty force-pushed the enhancement/configurabe_dataplane_log_format branch from aadd4f6 to f768d25 Compare November 6, 2025 16:59

ciarams87 reviewed Nov 6, 2025

View reviewed changes

internal/controller/nginx/config/base_http_config_test.go Outdated Show resolved Hide resolved

ciarams87 reviewed Nov 6, 2025

View reviewed changes

internal/controller/nginx/config/base_http_config.go Outdated Show resolved Hide resolved

ciarams87 reviewed Nov 6, 2025

View reviewed changes

internal/controller/nginx/config/base_http_config_template.go Outdated Show resolved Hide resolved

tataruty force-pushed the enhancement/configurabe_dataplane_log_format branch from f768d25 to a0ba554 Compare November 7, 2025 09:40

nginx-bot bot added the release-notes label Nov 7, 2025

ciarams87 requested changes Nov 7, 2025

View reviewed changes

internal/controller/nginx/config/base_http_config.go Outdated Show resolved Hide resolved

internal/controller/state/dataplane/configuration.go Outdated Show resolved Hide resolved

tataruty added 9 commits November 7, 2025 10:35

Adding nginx_proxy access_log format ability

177aacf

Make logFormat singular and add ability to turn logs off

5c2a2a2

Remove LoggingSettings structure

c46c1db

fix template

d84602a

fix review comments

f07cbe0

Removing NginxLogFormat for usability purpose

4a6f866

remove logFormat from dataplane as well

306062e

move template AccessLog to separate structure

cfb8bc5

remove unused vars

85cdb55

tataruty force-pushed the enhancement/configurabe_dataplane_log_format branch from 865c794 to 85cdb55 Compare November 7, 2025 10:35

ciarams87 reviewed Nov 7, 2025

View reviewed changes

internal/controller/nginx/config/base_http_config.go Outdated Show resolved Hide resolved

internal/controller/nginx/config/base_http_config_template.go Outdated Show resolved Hide resolved

small fix

7022c4f

Adding nginx_proxy access_log format ability #4102

Are you sure you want to change the base?

Adding nginx_proxy access_log format ability #4102

Uh oh!

Conversation

tataruty commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Checklist

Release notes

Uh oh!

codecov bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ciarams87 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ciarams87 commented Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tataruty commented Oct 17, 2025 •

edited

Loading

codecov bot commented Oct 17, 2025 •

edited

Loading

ciarams87 left a comment •

edited

Loading